sequenceDiagram
    participant GL as GameLogic
    participant NET as Network::update()
    participant NU as NetworkUtil
    participant CCL as GetCommandsFromCommandList()
    participant NCL as NetCommandList

    GL->>NET: frame advances
    NET->>NU: GetCommandID()
    alt "commandID >= 64000 AND new execution frame"
        NET->>NU: ResetCommandID()
    end
    NET->>CCL: collect game messages
    CCL->>NU: GenerateNextCommandID()
    CCL->>NCL: addMessage(cmd)
    NCL-->>NCL: sort by commandID
    NCL-->>NET: ordered command list

_{Reviews (6): Last reviewed commit: "Changed implementation." | Re-trigger Greptile}

Resetting the command id just delays the potential mismatch, right? Wouldn't it be better to sort the command ids properly, even if that rarely causes mismatches with retail clients?

Resetting the command id just delays the potential mismatch, right?

It's reset every N frames, so it's not just delayed.

When it's reset every 128 frames, isn't it possible that a packet from frame 127 comes in after it's already reset and then gets sorted wrong?

My current understanding is that the ids only matter per frame, not across different frames. But then I do not understand why it needs to be reset every 128 frames instead of every frame.

I noticed that at extremely high latency (beyond 64 frames runahead) the disconnect screen could be triggered by the reset somehow. I tried a ton of things, but I was unable to find out why that happens. That's why I made some changes to the implementation and only reset the command id when it gets close to overflowing.

When it's reset every 128 frames, isn't it possible that a packet from frame 127 comes in after it's already reset and then gets sorted wrong?

if (frame + m_runAhead > m_lastExecutionFrame)

This check should guard against that. If the check is inverted, you'll see wrong sorting once in a while.

My current understanding is that the ids only matter per frame, not across different frames. But then I do not understand why it needs to be reset every 128 frames instead of every frame.

If the runahead decreases the ids matter across multiple frames.

Also inquired with Chad Gippy:

If you have a monotonically increasing uint16_t counter that wraps from 65535 to 0, a direct comparison like:

if (a > b)

fails across the wrap boundary.

A common solution is to compare the difference modulo 2¹⁶ and interpret it as a signed value:

bool is_newer(uint16_t a, uint16_t b)
{
    return static_cast<int16_t>(a - b) > 0;
}

Examples:

is_newer(100, 50);      // true
is_newer(50, 100);      // false

is_newer(0, 65535);     // true  (wrapped by 1)
is_newer(65535, 0);     // false

The subtraction is performed modulo 65536:

0 - 65535 == 1

which becomes int16_t(1) and therefore indicates that 0 is newer than 65535.

Limitation

This only works if the counters are never more than half the range apart.

For a 16-bit counter:

maximum unambiguous distance = 32767

The value exactly half a range away (32768) is fundamentally ambiguous:

a = 0;
b = 32768;

You cannot tell whether a is newer than b or vice versa because both interpretations are equally valid on a circular counter.

A robust version is:

bool is_newer(uint16_t a, uint16_t b)
{
    uint16_t diff = a - b;
    return diff != 0 && diff < 0x8000;
}

This is essentially the same technique used for sequence numbers in networking protocols such as TCP, where wraparound is expected.

So what you could do for the non retail compatible sorting:

bool isCommandIdNewer(uint16_t newVal, uint16_t oldVal)
{
#if RETAIL_COMPATIBLE_NETWORKING
    return newVal > oldVal;
#else
    uint16_t diff = newVal - oldVal;
    return diff != 0 && diff < 0x8000;
#endif
}

This would require no manual ID resetting. Needs testing.